Add env to disable worker restart #10664
Conversation
| @@ -315,6 +316,10 @@ export default async function main( | |||
| ? vatTranscriptRetention !== 'operational' | |||
| : false; | |||
|
|
|||
| const restartWorkerOnSnapshot = processValue.getBoolean({ | |||
| envName: 'XSNAP_RESTART_ON_SNAPSHOT', | |||
There was a problem hiding this comment.
This should be documented in env.md.
There was a problem hiding this comment.
Since this config is fairly experimental, it is not currently documented anywhere. This is in line with similar XSNAP env.
There was a problem hiding this comment.
Then let's document it as experimental. I don't like having environment variable sensitivity that can only be discovered by reading the code.
| * @property {import('./chain-main.js').CosmosSwingsetConfig} swingsetConfig | ||
| * TODO refactor to clarify relationship vs. import('@agoric/swingset-vat').SwingSetConfig | ||
| * --- maybe partition into in-consensus "config" vs. consensus-independent "options"? | ||
| * (which would mostly just require `bundleCachePath` to become a `buildSwingset` input) | ||
| * @property {number} [maxVatsOnline] | ||
| * @property {boolean} [restartWorkerOnSnapshot] |
There was a problem hiding this comment.
This feels like a step in the wrong direction—I'd really like to move towards better categorization of launch inputs than just a flat object with way too many properties.
There was a problem hiding this comment.
I agree but right now passing the config itself for just picking a single consensus-independent option felt wrong too, especially because I didn't want this new option to be part of the cosmos driven config.
Also that same file used swingsetConfig to designate something else entirely, so removing stops the ambiguity
There was a problem hiding this comment.
That's fair, but the CosmosSwingsetConfig vs. SwingSetConfig problem still exists. I guess I'm fine with this refactoring if it doesn't lose the comment (or a variation thereof).
incidental
Description
Add an environment to control the restart of the vat worker when a snapshot is taken. This should have no impact on consensus, but we haven't actually tested this lately, and we used to have problems in the past.
Disable the forced restart in the loadgen CI test to exercise this potential source of divergence.
Security Considerations
None. A validator using this environment may fall out of consensus.
Scaling Considerations
Not restarting the worker has an impact on the performance of the worker over time, but given the limited amount of work the CI test does, it should not be noticeable.
Documentation Considerations
Since this config is fairly experimental, it is not currently documented anywhere. This is in line with similar XSNAP env.
Testing Considerations
Enabling this in CI
Upgrade Considerations
Part of chain software, but should not affect consensus.